-
Notifications
You must be signed in to change notification settings - Fork 150
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: improve contribution guide #486
Conversation
Signed-off-by: Damien Duportal <[email protected]>
CONTRIBUTING.adoc
Outdated
You need the following tools: | ||
|
||
* A bash compliant command line | ||
* link:http://man7.org/linux/man-pages/man1/make.1.html[GNU make] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nitpick: link
is optional and I like to extract URL at the top of the file:
:gnu-make-url: http://man7.org/linux/man-pages/man1/make.1.html
* {gnu-make-url}[GNU make]
Also, I think we should https
URL
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @ggrossetie ! I've added the https
link.
About the URL on top of the file: what is the rationale behind? I tend to do it when I repeat the URL on the file at least twice, but there might a good practise here that I don't know
Regarding the optional link
, I tend to it because we have a Markdown file (for use on the DockerHub) so I tend to be explicit to make it easier differentiating between both format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find it easier to update (potentially broken) links when they are all together at the top of the file. When in doubt, you can ctrl+click all of them since they are located at the top of your file and see if they are still working as expected. It's a bit more tedious when external links are scattered.
Regarding the optional link, I tend to it because we have a Markdown file (for use on the DockerHub) so I tend to be explicit to make it easier differentiating between both format.
👍🏻
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make sense! If you don't mind, I'll proceed with this PR (as it is a nitpick) and I'll send a subsequent PR with:
- replacing remaining http:// scheme by https:// (we're in 2024 so either these are dead links to remove or they can handle https:// scheme)
- Moving the links on top of the adoc documentations as described here
Co-authored-by: Guillaume Grossetie <[email protected]>
Signed-off-by: Damien Duportal <[email protected]>
This PR follows-up the discussion in https://github.com/asciidoctor/asciidoctor-reveal.js/pull/539/files#r1870061983.
It tries to improve the documentation over contribution guide, as they were "non explicit" areas which only were inside my brain. Healthiness of such a project needs maintainers to write down knowledge: this is the goal here (to avoid project being gate kept or depending on only one person).
It aims at improving the contributing experience by correctly explaining the expected changes of contributions.